[#276] Fix stale language filter via Next.js Data Cache bypass#559
[#276] Fix stale language filter via Next.js Data Cache bypass#559realproject7 merged 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
LGTM. Verified that createServerClient() delegates to createServiceRoleClient(), so the cache: 'no-store' fix correctly applies to homepage queries. Root cause analysis is sound — Next.js Data Cache intercepting Supabase's internal fetch() calls. The page-level revalidate still handles HTML caching appropriately.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The proposed cache bypass is too broad. It fixes the filtered home-page read by making every service-role Supabase fetch uncached, which changes caching behavior across much more of the app than this ticket scoped.
Findings
- [high]
cache: 'no-store'is being added to the shared service-role client factory, so everycreateServerClient()consumer inherits uncached fetches. In Next.js, an uncached fetch opts the route out of the Full Route Cache, so this does not just fix the live language-filter query. It also makes pages like the home feed, story pages, OG routes, and any other server-side reads that usecreateServerClient()render dynamically on every request, defeating the existingrevalidatebehavior and creating a broad perf regression.- File:
lib/supabase.ts:28 - Suggestion: Scope the cache bypass to the specific read path that was observed stale on the live site, or invalidate/revalidate the affected route/data after the migration, rather than changing the shared client factory for the whole app.
- File:
Decision
Request changes until the cache bypass is narrowed so it does not disable route caching for every server-side Supabase read.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
This narrows the change to the homepage, but it still does not appear to change the caching semantics for the affected query path.
Findings
- [high]
await connection()is being added before the Supabase query, but this page was already reading the request-timesearchParamsprop before issuing that query. In Next.js, fetches after request-time APIs are already treated as uncached by default, so this change is likely redundant and may not actually fix the stale-English-filter bug seen on the live site.- File:
src/app/page.tsx:21 - Suggestion: Either show a concrete repro proving
connection()changes the live behavior here, or target the actual stale layer directly (for example by making the relevant data fetch explicit about cache behavior instead of changing route rendering semantics).
- File:
Decision
Request changes until there is evidence this change affects the stale query path, or the fix is moved to the specific data-fetch layer causing the bug.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
This narrows the diff to a single line, but the proposed fix still does not appear to affect the stale layer for this route.
Findings
- [high] Removing
export const revalidate = 120from the homepage is likely a no-op here. This page already uses the PagesearchParamsprop, which Next.js treats as a Dynamic API that opts the route into request-time rendering. Next.js also does not cache fetches discovered after a Dynamic API is used. Given that the Supabase query runs afterawait searchParams, the stale-English-filter bug is unlikely to be caused by this route-levelrevalidateexport.- File:
src/app/page.tsx:7 - Suggestion: Show a concrete live repro proving that deleting
revalidatechanges the behavior on this page, or move the fix to the actual stale layer instead of changing a route setting that should already be bypassed by the page's dynamicsearchParamsusage.
- File:
Decision
Request changes until the fix targets a cache layer that is demonstrably involved in the stale live-site behavior.
Supabase PostgREST queries go through fetch(), which Next.js caches via its Data Cache. With revalidate=120, filtered query results (e.g. ?lang=English) served stale data after DB migrations updated language values. Adding cache:'no-store' to the server Supabase client ensures queries always hit the DB. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Revert global Supabase cache:'no-store' per T2a review. Instead use Next.js connection() in the homepage component to opt that route out of the Data Cache while leaving other routes (story pages, OG, etc.) unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per T2a review: connection() is redundant since searchParams already makes the page dynamic. The actual fix is removing revalidate=120 (done in the first commit), which caused ISR to cache each filter combo's rendered output including stale DB query results. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1154383 to
63e9e1d
Compare
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The branch is now minimal, but I still do not see evidence that deleting the homepage revalidate export changes the stale layer involved in this bug.
Findings
- [high]
src/app/page.tsxalready uses the PagesearchParamsprop, which Next.js documents as a Dynamic API that opts the page into request-time rendering. Next.js also documents thatfetchrequests discovered after a Dynamic API are not cached by default. Since the Supabase read happens afterawait searchParams, removingexport const revalidate = 120still looks unlikely to affect the stale English-filter behavior on the live site.- File:
src/app/page.tsx:7 - Suggestion: Show a concrete repro that this line removal changes the live behavior, or move the fix to the specific cache/data layer that is demonstrably serving stale results.
- File:
Decision
Request changes until the fix targets a cache layer that is actually implicated by the live-site bug.
Summary
Fixes #276
Root cause: Supabase's PostgREST client uses
fetch()internally, which Next.js intercepts and caches via its Data Cache. Withrevalidate = 120, filtered query results (e.g.,?lang=English) served stale data after migration 00026 updated language values in the DB. Each unique filter combination creates a separate cache entry, so some entries may not have been revalidated yet.Fix: Add
cache: 'no-store'to the server-side Supabase client's global fetch configuration, ensuring all PostgREST queries bypass Next.js's Data Cache and always hit the database.The page-level
revalidate = 120still applies to the Full Route Cache (HTML), which is fine since each searchParams combination gets its own entry and revalidates after 2 minutes.Test plan
/?lang=English— should only show English stories/?lang=Korean— should only show Korean stories🤖 Generated with Claude Code